Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GUI] CoinControl Change #2859

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Conversation

panleone
Copy link

@panleone panleone commented May 7, 2023

This PR extends the functionality of CoinControl change by adding shield addresses.
In particular:

  • In a transparent transaction user can select either a transparent or a sapling shield change
  • In a shield transaction user can only use a shield address as change (to preserve privacy)

(NB: with transparent transaction I mean a transaction in which inputs are transparent and analogous definition for shield transaction)

In addition this PR fixes #2245

@panleone panleone self-assigned this May 7, 2023
@panleone panleone added this to the 6.0.0 milestone May 7, 2023
Liquid369
Liquid369 previously approved these changes May 9, 2023
Copy link
Member

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK b05a3fa

Working as intended, the transaction below is for a custom change address.

https://zkbitcoin.com/tx/a4814fb9e12b8093e1fc854ddb222e5d113a0cefc9bab856769be5b10335d0e6
Screenshot 2023-05-09 at 2 50 49 PM

Disallows shielded change to be a transparent address

@Fuzzbawls
Copy link
Collaborator

Needs a rebase due to header file inclusion ordering, but otherwise this is functionally sound

@panleone
Copy link
Author

panleone commented Jun 3, 2023

rebased on master

Copy link
Member

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re tACK 1ec227f

Still working after rebase

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 1ec227f

@Fuzzbawls Fuzzbawls merged commit fec4cfe into PIVX-Project:master Jul 27, 2023
@Fuzzbawls Fuzzbawls modified the milestones: 6.0.0, 5.6.0 Feb 6, 2024
@panleone panleone deleted the qt_changeaddress branch March 23, 2024 09:00
panleone pushed a commit to panleone/PIVX that referenced this pull request Nov 10, 2024
* Automatically wake up select() when optimistic send was not used

But only when we know that we are actually inside select() and that it
currenlty is unlikely for it to have selected the node's socket for
sending. We accept race conditions here as the select() timeout
will ensure that we always send the data.

* Don't manually call WakeSelect() in CSigSharesManager::SendMessages

Not needed anymore

* Disable optimistic send in PushMessage by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Change address not working when doing Transparent send to Shield receiving address
3 participants